ct/l1: add cloud_topics_compaction scheduling group#29288
ct/l1: add cloud_topics_compaction scheduling group#29288WillemKauf merged 2 commits intoredpanda-data:devfrom
ct/l1: add cloud_topics_compaction scheduling group#29288Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new cloud_topics_compaction scheduling group to better isolate CPU resources for cloud topics compaction operations. The change introduces a dedicated scheduling group with a share count of 150, adds the necessary accessor method, and integrates it into the cloud topics level one compaction worker.
Changes:
- Added a new
cloud_topics_compactionscheduling group with 150 shares in the CPU scheduling system - Updated the compaction worker to accept and use the new scheduling group for its work loop
- Updated test fixtures to accommodate the new scheduling group parameter
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/v/resource_mgmt/cpu_scheduling.h | Defines and initializes the new cloud_topics_compaction scheduling group |
| src/v/cloud_topics/level_one/compaction/worker_manager.cc | Passes the new scheduling group to worker initialization |
| src/v/cloud_topics/level_one/compaction/worker.h | Adds scheduling group parameter to constructor and member variable |
| src/v/cloud_topics/level_one/compaction/worker.cc | Stores the scheduling group and wraps work loop execution with it |
| src/v/cloud_topics/level_one/compaction/tests/worker_manager_test.cc | Updates test to provide default scheduling group |
| src/v/cloud_topics/level_one/compaction/tests/scheduler_fixture.h | Updates test fixture to provide default scheduling group |
| src/v/cloud_topics/level_one/compaction/BUILD | Adds dependency on cpu_scheduling library |
WillemKauf
force-pushed
the
cloud_topics_compaction_sg
branch
from
04454c0 to
e715f68
Compare
|
Oops. Looks like we're capping out at 18 scheduling groups max, and redpanda/bazel/thirdparty/seastar.BUILD Lines 75 to 79 in 2a9fc3b Is there any reason we can't kick the can down the road by bumping this? We should also probably start investigating use of scheduling supergroups: |
dotnwat
left a comment
dotnwat
left a comment
There was a problem hiding this comment.
whats the status on this are we planning to make this change?
🤷 there was a brief discussion on scheduling groups here. Probably something to raise again with the team in a meeting. |
|
Closing in favour of re-using the existing |
WillemKauf
force-pushed
the
cloud_topics_compaction_sg
branch
from
e715f68 to
ed3c697
Compare
| int_flag( | ||
| name = "scheduling_groups", | ||
| build_setting_default = 18, | ||
| build_setting_default = 19, |
There was a problem hiding this comment.
Maybe I missed a discussion on this, but should we give ourselves headroom so this doesn't need to get bumped each time we add a scheduling group? Like we jump to 24 or 32?
There was a problem hiding this comment.
My thinking was to leave it maxed out just so people really have to think about adding more before they do so.
CI test resultstest results on build#80107
|
4 participants
Backports Required
Release Notes